Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instantaneous CAM-SIMA history infrastructure #274

Merged
merged 97 commits into from
Oct 25, 2024

Conversation

peverwhee
Copy link
Collaborator

@peverwhee peverwhee commented Jul 8, 2024

Tag name (required for release branches):
Originator(s): peverwhee, gold2718

Summary (include the keyword ['closes', 'fixes', 'resolves'] and issue number):

  • Implement instantaneous history output in CAM-SIMA using the new modular history code (addresses Implement diagnostic capability #29)
  • Update the standard name inverse_exner_function_wrt_surface_pressure to reciprocal_of_dimensionless_exner_function_wrt_surface_air_pressure

Describe any changes made to build system:
A cime_config/hist_config.py: New namelist parsing for history configurations
M cime_config/buildlib: Add new history directories
M cime_config/buildnml: Incorporate hist_config.py
M cime_config/atm_in_paramgen: Incorporate hist_config.py

Describe any changes made to the namelist: Can now use CAM-SIMA history namelist - see https://escomp.github.io/CAM-SIMA-docs/usage/history/

List any changes to the defaults for the input datasets (e.g. boundary datasets): none

List all files eliminated and why:
D src/control/filenames.F90
-renamed to src/utils/cam_filenames.F90

List all files added and what they do:
A src/control/cam_physics_control.F90
-split cam_control_mod to avoid circular dependency
A src/utils/cam_filenames.F90
-renamed from control/filenames.F90
A src/history/cam_hist_file.F90
A src/history/cam_history.F90
A src/history/cam_history_support.F90
-core CAM-SIMA history infrastructure
A test/unit/test_hist_config.py
-test hist_config.py (namelist parsing)

List all existing files that have been modified, and describe the changes:
(Helpful git command: git diff --name-status development...<your_branch_name>)
M src/control/cam_comp.F90
-add calls to new history infrastructure
M src/control/cam_control_mod.F90
-split to into cam_control_mod and cam_physics_control
M src/control/runtime_opts.F90
-add call to history namelist reader
M src/control/cam_initfiles.F90
-update use statement
M src/cpl/nuopc/atm_comp_nuopc.F90
-add rstwr and nlend to cam_timestep_final calls, add do_history_write logical
M src/data/registry.xml
M src/dynamics/se/dyn_comp.F90
M src/dynamics/mpas/dyn_comp.F90
-update exner and vertically_integrated_* standard names
M src/dynamics/utils/hycoef.F90
-enable history coordinates
M src/utils/cam_abortutils.F90
-fix bug in cam_register_open_file
M src/utils/cam_field_read.F90
-code cleanup
M src/utils/cam_grid_support.F90
-update to enable split history files
M src/utils/cam_pio_utils.F90
-only register created and opened files if we're writing to them
M src/utils/cam_time_coord.F90
M src/utils/time_manager.F90
-update use statement
M src/utils/string_utils.F90
-add functions date2yyyymmdd and sec2hms
M test/run_unit_tests.sh
-add new hist_config test

If there are new failures (compare to the test/existing-test-failures.txt file),
have them OK'd by the gatekeeper, note them here, and add them to the file.
If there are baseline differences, include the test and the reason for the
diff. What is the nature of the change? Roundoff?

derecho/intel/aux_sima:

derecho/gnu/aux_sima:

If this changes climate describe any run(s) done to evaluate the new
climate in enough detail that it(they) could be reproduced:

Steve Goldhaber and others added 30 commits July 29, 2022 20:38
Implement read of new history namelist
Added unit tests for CAM history unit tests
Added multiplier parser and utiliity unit tests
Remove redundant filenames.F90
Finish some checking functions
Added error strings to command parser functions
First pass of HistoryConfig class
Start on testing history configuration parsing
Progress on hist config parsing
Still working on new format, start with get_entry
Finish parse_hist_config_file
First pass at some AMWG history configuration
Updated from focus meeting
Fix location of history unit test sample files
Take out debug exception
Test passes but output file is not yet correct.
Full test of flat user_nl_cam passes
Updated namelist names to begin with hist_
Rename user_nl_cam history entries to begin with hist_
Progress on multi-level history config
Reconfigured class structure, flat and multi tests pass
Added history configuration parsing and writing to buildnml
Add diagnostic element to registry
Updated CMakeLists.txt for new share code file layout
…ing gravit standard name. Making etamid visible via metadata file.
Copy link
Member

@jimmielin jimmielin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for requesting my review @peverwhee! This looks great, I only have a few minor comments, mostly on the total energy standard names that we discussed for a separate snapshot issue (ESCOMP/CAM#1141), since they were updated in this PR. My comments reflect the current state of the CAM snapshots that do not output the _using_dycore_formula variants of te_ini and te_cur. #296 will make a more thorough fix.

src/data/registry.xml Outdated Show resolved Hide resolved
src/data/registry.xml Outdated Show resolved Hide resolved
src/data/registry.xml Outdated Show resolved Hide resolved
src/data/registry.xml Outdated Show resolved Hide resolved
src/history/cam_hist_file.F90 Outdated Show resolved Hide resolved
@peverwhee peverwhee requested a review from nusbaume September 16, 2024 20:23
cime_config/hist_config.py Outdated Show resolved Hide resolved
cime_config/hist_config.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks a lot better! Just some last final suggestions and requests. Of course if you think these would be easier to push off until a later date (e.g. via a new issue) then just let me know. Thanks!

cime_config/hist_config.py Outdated Show resolved Hide resolved
cime_config/hist_config.py Show resolved Hide resolved
cime_config/hist_config.py Outdated Show resolved Hide resolved
cime_config/hist_config.py Outdated Show resolved Hide resolved
test/unit/test_hist_config.py Outdated Show resolved Hide resolved
src/history/cam_hist_file.F90 Show resolved Hide resolved
src/history/cam_hist_file.F90 Show resolved Hide resolved
src/history/cam_hist_file.F90 Show resolved Hide resolved
test/run_unit_tests.sh Show resolved Hide resolved
src/history/cam_history.F90 Show resolved Hide resolved
@peverwhee peverwhee requested a review from nusbaume September 25, 2024 20:50
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great now! I have one last request, but it doesn't need a re-review. Thanks again for getting this implemented @peverwhee!

return (None, errmsg)
# end if
# Check multiplier
tokens = [x.strip() for x in str(entry.strip()).split('*')]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think entry is already a string, so no need to convert it here. Also I don't think there is any need to strip twice:

Suggested change
tokens = [x.strip() for x in str(entry.strip()).split('*')]
tokens = [x.strip() for x in entry.split('*')]

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleaned up! thanks!

Copy link
Collaborator

@mwaxmonsky mwaxmonsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this one @peverwhee! Looks good to me!

Copy link
Collaborator

@cacraigucar cacraigucar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not reviewed the actual code here (CAM history has always been pretty much a black box for me). My review mainly has been to use what has been implemented and make sure it works (as it does). Let me know if you want me to review the Fortran code, otherwise I am giving my approval (pending the changes I've requested for the submodules locations).

.gitmodules Outdated Show resolved Hide resolved
.gitmodules Outdated Show resolved Hide resolved
@peverwhee peverwhee merged commit e69640a into ESCOMP:development Oct 25, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants